feat(photos): Add Google Photos import via Picker API#356
Conversation
|
This relates to #207 |
|
Undrafted to get review from Copilot, apologies code owners |
There was a problem hiding this comment.
Pull request overview
Replaces the deprecated Google Photos Library API integration with the Google Photos Picker API flow, adding a user-driven photo selection popup and a background-job-based import into Nextcloud Files.
Changes:
- Added a Photos section in personal settings with Picker-session creation/polling and import progress UI.
- Introduced a new
GooglePhotosAPIServiceplusImportPhotosJobto orchestrate downloads from Picker sessions. - Added new routes/controllers/config plumbing and a completion notification.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/PersonalSettings.vue |
Adds Photos picker/import UI, polling loops, and OAuth popup state refresh. |
src/components/AdminSettings.vue |
Updates admin setup instructions for required Google APIs. |
lib/Service/GooglePhotosAPIService.php |
Implements Picker session management and background import/download orchestration. |
lib/BackgroundJob/ImportPhotosJob.php |
Queued job wrapper that delegates to the Photos import service. |
lib/Controller/GoogleAPIController.php |
Adds endpoints for picker sessions, import start, and import progress info. |
lib/Controller/ConfigController.php |
Adds Photos scope, GET /config endpoint, and cancel-import wiring. |
lib/Notification/Notifier.php |
Adds import_photos_finished notification linking to the import directory. |
lib/Settings/Personal.php |
Adds photo_output_dir initial state defaulting to /Google Photos. |
appinfo/routes.php |
Registers the new config/picker/import routes. |
appinfo/info.xml |
Bumps app version to 4.3.2. |
package-lock.json |
Updates engines metadata (Node/NPM) to match package.json. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
- Clear driveImportLoop in beforeUnmount() to prevent Drive polling timer leak when component is destroyed (nextcloud#92) - Move pickerPollTimer clearance into onImportPhotos() success handler; restart polling on import request failure so user is not stuck with an active session and no way to trigger import (nextcloud#93) - Delete newly created file in downloadAndSaveFile() when fopen throws LockedException or returns false, preventing zero-byte placeholder files from being left behind on early failure paths (nextcloud#94) Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Mirror the same defensive pattern used in startImportPhotos(): if the stored picker_session_queue value cannot be decoded to an array (e.g. config corruption), fall back to an empty array rather than passing a non-array value to array_shift(). Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
- Cancel active Drive and Photos imports on user disconnect so background jobs don't re-queue against deleted tokens (nextcloud#106) - Add startingPhotoImport guard in pollPickerSession() to prevent concurrent /import-photos POST requests across poll ticks (nextcloud#107) Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
- Validate pollInterval from pollingConfig with Number.isFinite() and fall back to 5000ms to prevent NaN-driven 0ms poll loop (nextcloud#110) - Reset importing_drive/photo and *_import_running flags on disconnect so re-queued jobs exit cleanly against deleted tokens (nextcloud#111) - Null pickerPollTimer after clearInterval in onCancelPickerSession() to keep polling-restart guards correct (nextcloud#112) Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
- Fix php-cs: remove duplicate /** in GoogleDriveAPIService docblock - Remove redundant axios config fetch from handleOAuthMessage; loadData() already refreshes page state after OAuth (thread 62) - Rename last_import_timestamp -> last_photo_import_timestamp for consistency with last_drive_import_timestamp (thread 63) - Move importing_*/import_running flag resets into cancelImport() for both services; remove the duplicate setValueString calls from the disconnect path in ConfigController (thread 64) - cancelImport() now also deletes all queued picker sessions from picker_session_queue so no stale sessions remain in Google (thread 65) - Consolidate duplicate deletePickerSession() calls (success + error) into a single call at the top of the terminal-state block (thread 66) Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
…File() Prevents exceptions thrown during file cleanup (on LockedException, fopen()=false, or simpleDownload error) from propagating out of the helper and aborting the import job. The method now consistently returns null on any failure path. Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com> Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
|
Apologies for the entire history looking like its been recommitted, I forgot a sign-off in one of my commits and didn't realise that rebasing my sign-offs would make GitHub show my commits as if they were all newly pushed together |
lukasdotcom
left a comment
There was a problem hiding this comment.
No problem on the commit history I can tell which commits are actually new. I missed these two small things, but once that is fixed I can merge this.
Co-authored-by: Lukas Schaefer <lukas@lschaefer.xyz> Signed-off-by: Ahsan <61637519+AhsanIsEpic@users.noreply.github.com>
Signed-off-by: Ahsan Ahmed <61637519+AhsanIsEpic@users.noreply.github.com>
|
I think this is ready now to merge |
|
Thanks @AhsanIsEpic for the contribution it is now merged and will be in the next release. |
Hi @lukasdotcom do you have an ETA for the next release. Waiting for this feature to be available. Thanks a lot! |
Hi @JonasSchubert, I'm creating a release right now so ETA would be today. |
Closes #357
Summary
The Google Photos Library API is deprecated and returns a
403 Forbiddenerror for new OAuth clients. This PR replaces it entirely with the Google Photos Picker API, which requires users to explicitly select photos in a Google-hosted popup before Nextcloud can access any data.This replaces
photoslibrary.readonly. Existing connected users must disconnect and re-authenticate to grant the new scope. Thecan_access_photosflag is written to user config during the OAuth redirect and controls visibility of the Photos section in personal settings.Admin setup: In the Google Cloud Console, enable the "Google Photos Picker API" (not the deprecated Photos Library API).
Changed Files
appinfo/routes.phpSix new routes:
GET/configconfig#getConfigPOST/picker-sessiongoogleAPI#createPickerSessionGET/picker-sessiongoogleAPI#getPickerSessionDELETE/picker-sessiongoogleAPI#deletePickerSessionPOST/import-photosgoogleAPI#importPhotosGET/import-photos-infogoogleAPI#getImportPhotosInformationlib/BackgroundJob/ImportPhotosJob.php(new)A
QueuedJobthat delegates toGooglePhotosAPIService::importPhotosJob(). Re-queued by the service until all picked items are downloaded.lib/Controller/ConfigController.phpPHOTOS_SCOPEconstantINT_CONFIGSextended withnb_imported_photos,last_import_timestamp,photo_import_job_last_startgetConfig()endpoint (GET /config): returnsuser_nameanduser_scopes— used by the OAuth popup redirect to refresh the parent settings page state without a full reloadoauthRedirect(): writescan_access_photosto the storeduser_scopesarraysetConfig(): disconnect path now deletesuser_scopes; cancel-import path routes throughGooglePhotosAPIService::cancelImport()lib/Controller/GoogleAPIController.phpFive new controller actions:
createPickerSession()—POST /picker-session: creates a Picker session, returnsid,pickerUri(with/autocloseappended), andpollingConfiggetPickerSession()—GET /picker-session?sessionId=: polls session state, returnsmediaItemsSetdeletePickerSession()—DELETE /picker-session?sessionId=: deletes session; only clears the storedpicker_session_idconfig key when it matches the deleted session, so cancelling a UI session cannot corrupt an active importimportPhotos()—POST /import-photos: callsstartImportPhotos(), returnstargetPath(andqueued: trueif an import is already running)getImportPhotosInformation()—GET /import-photos-info: returnsimporting_photos,nb_imported_photos,last_import_timestamp,nb_queued_sessionslib/Notification/Notifier.phpNew
import_photos_finishednotification: correctly pluralised via$l->n(), links to the import folder in Files, uses the app's dark icon.lib/Service/GooglePhotosAPIService.php(new)Core Picker API client and import orchestrator:
Session management
createPickerSession()—POST /v1/sessions; does not writepicker_session_id(onlystartImportPhotos()does, to avoid overwriting an active import session when the user queues a second picker)getPickerSession()—GET /v1/sessions/{id}deletePickerSession()—DELETE /v1/sessions/{id}; only clears storedpicker_session_idwhen it matchesImport orchestration
startImportPhotos(): validates$sessionId, creates the output folder, stores session ID, resets counters, enqueuesImportPhotosJob. If an import is already running, appends topicker_session_queueand returnsqueued: trueinsteadimportPhotosJob(): runs under user + filesystem scope; concurrent-run guard viaphoto_import_running+photo_import_job_last_start; delegates toimportFromPickerSession(); on finish sends notification + deletes session; on partial run re-queues itself. On successful completion with a non-empty queue, transitions atomically to the next queued session (never writesimporting_photos=0transiently)importFromPickerSession(): paginatesGET /v1/mediaItems(100/page); per-item dedup vianodeExists+IFilesMetadataManagermetadata (stores Google item ID asintegration_google_photo_id); honours a 500 MB per-run cap and persistsphoto_next_page_tokenso subsequent job runs resume where they left offcancelImport(): removes the job, deletes the active picker session, clears the queueDownload (
downloadPickerItem()→GoogleAPIService::downloadAndSaveFile())=dfor images,=dvfor video); mtime set fromcreateTimeGoogleAPIService::downloadAndSaveFile()lib/Settings/Personal.phpReads
photo_output_dirfrom user config (defaulting to/Google Photos) and injects it into theuser-configinitial state.src/components/AdminSettings.vueUpdates setup instructions to reference the Google Photos Picker API in the Google Cloud Console.
src/components/PersonalSettings.vueNew Photos section:
lib/Service/GoogleAPIService.phpNew
downloadAndSaveFile()public method: creates a file in a folder, streams a download viasimpleDownload(), sets mtime, and cleans up (deletes the empty file) on all failure paths includingLockedExceptionandfopen()returningfalse. Shared by bothGoogleDriveAPIServiceandGooglePhotosAPIService.Picker flow
POST /picker-sessionand opens it in a popup (noopener,noreferrer)GET /picker-sessionevery ~4 s; whenmediaItemsSetbecomestrue, the import is triggered automaticallyImport progress UI
NcLoadingIconspinnernb_imported_photos === 0and cron hasn't run yetnb_queued_sessions > 0Other
/Google Photos)BroadcastChannel('integration_google_oauth')exclusively for the success handshake;popupSuccess.jsposts to the channel and closes. All popup windows usenoopener,noreferrersrc/popupSuccess.jsUses
BroadcastChanneldirectly (no opener fallback): creates the channel, posts{ username }, closes the channel, then closes the window unconditionally.Even though this implementation currently works. It was developed with AI assistance (GitHub Copilot / Claude Sonnet) and should be treated as draft quality. It works end-to-end in a local Docker environment but has not been through a thorough human review or extensive testing.
Feedback welcome on anything including: